-
Notifications
You must be signed in to change notification settings - Fork 145
WIP: JDK-8091967: Add a MappedList Implementation #499
base: develop
Are you sure you want to change the base?
Conversation
Please read the CONTRIBUTING guidelines, specifically the Before submitting a pull request section. Since this is a new feature that adds new API, it will need prior discussion on the mailing list as to whether it is suitable to bring this into the core of javafx.base. Marking this as "WIP" since it is premature to review it. |
@kevinrushforth first thank you for pointing me to the CONTRIBUTING guidelines. I'm a bit unsure how I should proceed now that you've marked the PR as WIP. I understand that I need to sign the OCA, but apart from that I don't know how to proceed. You mentioned that the new API needs prior discussion on the mailing list. When "triggering" this discussion is it sufficient to write the JBS-ID and the description text provided in this PR? Is this apart from the OCA the only point left to do for now? I must confess that compared to other projects the barrier to provide new features to openjfx is quite difficult and time consuming for the one providing those. |
Your question really boils down to the more general "How do I get a new feature into JavaFX?". You commented that the barrier seems a bit higher than you were expecting. For new features, this is intentional. The general guidelines for new features are covered here, which was linked from the CONTRIBUTING guidelines mentioned above, so I assume you've already read it. You asked what the next steps were, which suggests that the guidelines could be clearer. I took a stab at adding some more detail that should eventually be added to the CONTRIBUTING guidelines. Please see Note: New Features in JavaFX. That note should be taken as a general guideline on feature development, and not a comment on your specific proposal. I do have some concerns about the value proposition of this proposed feature, but the right place to discuss that is the mailing list. To answer your other question, yes we also need a signed OCA from you before we can review this or any contribution. |
Thank you for clarifying the process of adding new features to JavaFX. What is still missing in my opinion is how the JBS issues are related to the whole process. The idea why I even opened this PR is mainly because of the linked JBS issue. Let me elaborate: I'm a huge fan of map and fold operations like Java supports with its streaming API, which was introduced in Java 8. Therefore somewhere around a year ago I searched for a way to apply map and fold operations to an Because the JBS issue was already quite old and didn't have a lot of feedback, I thought about implementing my own solution to problem based on the Now after reading your comments I'm asking myself what do JBS issues represent, and why not discuss the value of a requested feature inside the corresponding JBS issue? Previously I thought they indicate approved abstract feature requests and and confirmed bug reports, but according to your comments this doesn't seem to be the case. |
I can see your point about the open JBS Enhancement request, so it seems that there might be still something missing in our README and/or CONTRIBUTING docs that could help explain this. I can't immediately think of how to improve it, but I'll think about it. For bugs, it's pretty straight-forward, a JBS issue of type It isn't as straight-forward for enhancements. Unless something is clearly in the "no this will never make any sense" category, we don't usually close an As for discussing it in the JBS issue itself, the main reason this won't work is that most developers do not have write access to JBS. I suppose that an alternative approach would be to use a linked GitHub issue (I note issue #303 is the one for this bug), and have the discussion there, but even in that case, the initial high-level proposal, the "I would like to discuss implementing feature XYZ", needs to happen on the mailing list (you can certainly reference the JBS issue and the GitHub issue in that email as supporting documentation). |
As announced in this message, the official This sandbox repository is being retired on 1-Oct-2019. You will need to migrate your WIP pull request to the openjdk/jfx repo if you want to continue working on it. Here are instructions for migrating your pull request. The updated CONTRIBUTING.md doc has additional information on how to proceed. Once you have done this, it would be helpful to add a comment with a pointer to the new PR. NOTE: since this is a feature / enhancement request it needs to be discussed on the openjfx-dev mailing list if you want it to be considered. The new openjdk/jfx repo will be open for pull requests on Wednesday, 2-Oct-2019. I will send email to the openjfx-dev mailing list announcing this. |
This PR adds a
MappedList
class (and corresponding tests) to OpenJFX.The
MappedList
class takes a sourceObservableList
and a mapperFunction
as its arguments and applies the mapper function on each object inside the source list. The result is then returned as the content of theMappedList
.In case no mapper function is applied (i.e.
mapper = null
), theMappedList
acts as an empty list with a size of zero.Corresponding GitHub issue: #303
Corresponding JBS issue: https://bugs.openjdk.java.net/browse/JDK-8091967
The code is based on my implementation done in: https://github.com/PhoenicisOrg/javafx-collections
I'm not sure about the code standards for this project therefore I've just done some slight changes to make the classes (
MappedList
andMappedListTest
) fit better together with the otherTransformationList
implementations. Therefore if you have any requirements for the code feel free to tell me so that I can make the necessary changes, otherwise I hope that my implementation finds approval.